Skip to content

fix(ctb): Remove minGas revert in relayMessage#5508

Merged
OptimismBot merged 2 commits intodevelopfrom
clabby/ctb/xdm-min-gas-fix
Apr 24, 2023
Merged

fix(ctb): Remove minGas revert in relayMessage#5508
OptimismBot merged 2 commits intodevelopfrom
clabby/ctb/xdm-min-gas-fix

Conversation

@clabby
Copy link
Contributor

@clabby clabby commented Apr 20, 2023

Overview

Removes the revert in relayMessage when the minimum gas threshold is not met.

Metadata
Fixes CLI-3870

@changeset-bot
Copy link

changeset-bot bot commented Apr 20, 2023

⚠️ No Changeset found

Latest commit: f758778

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@clabby
Copy link
Contributor Author

clabby commented Apr 20, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@clabby clabby force-pushed the clabby/ctb/call-with-min-gas-fix branch from 941664a to 3b6fcbf Compare April 20, 2023 18:06
@clabby clabby force-pushed the clabby/ctb/xdm-min-gas-fix branch 4 times, most recently from 7bbc512 to 361d372 Compare April 20, 2023 19:28
@maurelian
Copy link
Contributor

Fixes CLI-3870

@semgrep-app
Copy link
Contributor

semgrep-app bot commented Apr 23, 2023

Semgrep found 2 unchecked-type-assertion findings:

  • op-bindings/bindings/l2crossdomainmessenger.go: L494
  • op-bindings/bindings/l1crossdomainmessenger.go: L525

Unchecked type assertion.

Created by unchecked-type-assertion.

@clabby clabby marked this pull request as ready for review April 23, 2023 20:13
@clabby clabby requested a review from a team as a code owner April 23, 2023 20:13
@clabby clabby force-pushed the clabby/ctb/call-with-min-gas-fix branch 2 times, most recently from c136b42 to 48ef885 Compare April 24, 2023 17:36
@clabby clabby force-pushed the clabby/ctb/xdm-min-gas-fix branch 2 times, most recently from 67d5326 to cd984d0 Compare April 24, 2023 17:42
@clabby clabby force-pushed the clabby/ctb/call-with-min-gas-fix branch from 48ef885 to 1c61224 Compare April 24, 2023 17:55
@clabby clabby force-pushed the clabby/ctb/xdm-min-gas-fix branch from cd984d0 to 2bf225a Compare April 24, 2023 17:56
Base automatically changed from clabby/ctb/call-with-min-gas-fix to develop April 24, 2023 18:32
@netlify
Copy link

netlify bot commented Apr 24, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit f758778
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/6446d0ebdd4dc7000807c7af

@mergify
Copy link
Contributor

mergify bot commented Apr 24, 2023

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Apr 24, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

@refcell
Copy link
Contributor

refcell commented Apr 24, 2023

@Mergifyio unqueue

@mergify
Copy link
Contributor

mergify bot commented Apr 24, 2023

unqueue

☑️ The pull request is not queued

@mergify mergify bot removed the on-merge-train label Apr 24, 2023
@OptimismBot OptimismBot merged commit 3006ef0 into develop Apr 24, 2023
@OptimismBot OptimismBot deleted the clabby/ctb/xdm-min-gas-fix branch April 24, 2023 19:14
@mergify
Copy link
Contributor

mergify bot commented Apr 24, 2023

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Apr 24, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot removed the on-merge-train label Apr 24, 2023
seolaoh added a commit to kroma-network/kroma that referenced this pull request Aug 9, 2023
`CrossDomainMessager` whereby `relayMessage` naively reset the
`xDomainMsgSender` after a call. The issue here was that a nested call would
reset this value to the `Constants.DEFAULT_L2_SENDER` which deviates from the
expected behavior that the `xDomainMsgSender` is set to the `_sender` for the
entirety of the subcall.
This change sets a message as failed upon re-entry to the `relayMessage`
function by checking the value of `xDomainMsgSender`. We then also remove the
`reentrancyLocks` from storage and don't have to worry about re-entrency here
since any subcall through the `SafeCall` lib to the XDM `relayMessage` will be
caught and recorded as failed by the aforementioned `xDomainMsgSender` check.
Also removes the revert in `relayMessage` when the minimum gas threshold is not
met.

See:
  - ethereum-optimism/optimism#5475
  - ethereum-optimism/optimism#5444
  - ethereum-optimism/optimism#5493
  - ethereum-optimism/optimism#5508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants